If you don't need the constructor for anything else, you can set the shorthand:

export class SavedObjectsInstaller extends React.Component {
    state = {
        isInstalling: false,
        installStatus: INCOMPLETE,
    };
}

For consistency installStatus should be a boolean like isInstalling (and named isComplete).

If status only has two modes, I think it would be better to make it a boolean.

I prefer const since it gives the assurance that this variable won't be reassigned later. If you use a ternary you can avoid let.

Instead of having two different checks for error (errors.length === 0 and errors.length > 0) maybe just have one const hasError = errors.length > 0.

This way they won't somehow come out of sync in a future change.

I think we made a mistake by coupling rest clients to Angular in the past. Instead they should have been framework-agnostic static functions.

If we keep coupling new code (in this case React) to Angular we make it harder to switch away from Angular, and increase technical debt.

It's easier to use an existing client but I don't think the alternative is much more difficult. You don't have to re-implement the entire savedObjectsClient - just create a starting point with the endpoints you need.

Afaict this is all you need:

export function bulkCreate(savedObjects, { overwrite }) {
  return kfetch({
    pathname: `/api/saved_objects/_bulk_create`,
    method: 'POST',
    query: {
      overwrite
    },
    body: JSON.stringify(savedObjects)
  });
}

@epixa Do we already have a strategy for consuming Angular services from outside Angular - and how to migrate them? I've already spent a fair amount of time de-angularizing some services and it would be beneficial if everyone were aligned. If we don't have anything I'm open for doing a write-up of the approach I've used, and how I see us moving forward.

I do not think this is a problem and is exactly how we are interacting with saved objects in other react components. From react's perspective, it is just an injected function. Eventually, the savedObjectClient will be decoupled from angular but all saved object interaction should go through the savedObjectClient.

but all saved object interaction should go through the savedObjectClient

Maybe I don't know enough about the savedObjectClient. What does it provide wrt bulkCreate, that a static method can't do?

The savedObjectClient converts the results into savedObject classes and camel cases the object keys. But more importantly, it puts all calls to the saved object API in a single place so if the API ever changes, only the savedObjectClient has to be updated instead of hunting down lots of segregated fetches.

But more importantly, it puts all calls to the saved object API in a single place so if the API ever changes, only the savedObjectClient has to be updated instead of hunting down lots of segregated fetches

Totally agree. IMHO it is okay temporarily to have two "clients": the old Angular client, and a new framework agnostic service. Over time the Angular client could even call the framework agnostic client, and eventually the Angular client would be dropped. That's just my view, but interested to hear what other ideas are floating around (ping @epixa ;) )

Btw. I think it's fine to leave it as is. Just my opinion on how to do it going forward.

Not in this PR, but I'd rather just remove the angular dependency from the saved object client we have today. It shouldn't be terribly difficult, we'd just need to update any references to the saved object client from angular code to invoke $scope.$apply() after the call.

Is this just for debugging purposes? I assume this should be dynamically generated, so that when the mapping changes this doesn't get out of sync.

Storing the index pattern saved object like this is on purpose. If the mappings change between versions then this will need to be updated - just like the other saved objects will need to be updated in the event that field names change and so on.

True, we still need to update the dashboards if a field is renamed or removed. But up until now we've been able to make backwards compatible changes to the schema (like adding a new field) that didn't require us to update anything in Kibana.

What is the advantage of storing the static object, instead of creating it dynamically?

The advantage is being able to link the other saved objects directly to the index-pattern by id. How does the import work now, how do the saved objects link to the index-pattern? Are the users prompted with a list of index-patterns that match the index-pattern title?

The user might access APM UI (which also needs the index pattern) before opening the Getting Started Guide. We therefore need a single solution for creating the index pattern, so we don't end up with duplicate saved objects files that needs to be updated whenever apm-server schema changes.

I outlined three different solutions here: elastic/apm-server#991

If you have some alternative solutions I'm all ears :)

@nreese You might be right. The thinking behind having two buttons ["Import index pattern", "Import index pattern and dashboards"] is that users will need to manually update the index pattern whenever they update their version of the stack. However, clicking "Import index pattern and dashboards" will also overwrite existing visualizations (AFAIK, correct me if I'm wrong), which can be annoying.
But, we might be overthinking this. Thoughts?

Bit more context: The reason for mentioning "Index pattern" specifically is because we have this call out to the user in the UI: #20077
Having said that, I like what you did with abstracting all the details away by writing "Import Kibana objects". Maybe that's much better. We can have the details on a documentation page, or something. @formgeist @sqren Thoughts?

+1 for just having one option and calling it "Import Kibana objects".

Fine with me. If this causes issues for the users we can iterate on it.

Agreed, let's go with one button. Thanks for input @nreese.

Copy suggestion for the setup instructions step:

Title:
Kibana objects

Description:
Imports index pattern, visualizations and pre-defined dashboards. Index pattern is required for some features in the APM UI.

Button copy:
Load/Import Kibana objects

Instead of disabling eslint, I think the JSON should be moved to JSON-files. That will also make it easier to update.

These can not move to a json file since they need javascript logic to set the apm index pattern title

Perhaps this comment was meant for get_index_pattern_saved_object.js?

Have you tried just doing testSubjects.click? The move logic is internal:

    async click(selector, timeout = defaultFindTimeout) {
      log.debug(`TestSubjects.click(${selector})`);
      return await retry.try(async () => {
        const element = await this.find(selector, timeout);
        await remote.moveMouseTo(element);
        await element.click();
      });
    }

Two awaits here. Also, have you tried just extending the timeout? You can do something like:

await testSubjects.exists('loadSavedObjects_success', 10000) if the default timeout of 1 second is not long enough.

Another option is to just do await testSubjects.find('loadSavedObjects_success');

That should have an internal retry since it assumes it's supposed to exist:

    async _ensureElementWithTimeout(timeout, getElementFunction) {
      try {
        const remoteWithTimeout = remote.setFindTimeout(timeout);
        return await retry.try(async () => {
          const element = await getElementFunction(remoteWithTimeout);
          // Calling any method forces a staleness check
          element.isEnabled();
          return element;
        });
      } finally {
        remote.setFindTimeout(defaultFindTimeout);
      }
    }

nm, I see you already got rid of the second await in a follow up commit.

cc @spalger - I think this is a deficiency to having non-squashed commits. How do you keep up with all the changes from commit to commit to commit?

For instance I reviewed before this commit. I wanted to see the functional tests added so I went to that commit and added the comment. But the next commit fixed it. So what would your flow be? Start at commit x and review each one, x -> x + 1 -> x + 2 ... ? What if an x + n commit fixed something you commented on prior? Just go back and delete it?

Or do you review all commits from one commit onward but not the pull merges? Doesn't seem possible to do this in the git UI. It's either review all changes, or review commit by commit.

With "async/await everything" I've started to write:

const bulkCreateMock = async () => { 
  throw new Error('simulated bulkRequest error') 
}

It is not much shorter but I've come to like it. Use it if you want :)

Might be nice to just have a config.getOrDefault(TITLE_KEY, DEFAULT_TITLE) method. Seems like I've seen this same conditional in lots of places, not just in this PR.